Finish milestone #178
Finish milestone #178
Conversation
# Conflicts: # gh_review_project/review_project.py # gh_review_project/test/test.json
james-bruten-mo
left a comment
james-bruten-mo
left a comment
There was a problem hiding this comment.
Just a couple of suggestions
gh_review_project/review_project.py
Outdated
| import shlex | ||
| from collections import defaultdict | ||
|
|
||
| project_id = 376 |
There was a problem hiding this comment.
| project_id = 376 | |
| PROJECT_ID = 376 |
gh_review_project/review_project.py
Outdated
| from collections import defaultdict | ||
|
|
||
| project_id = 376 | ||
| project_owner = "MetOffice" |
There was a problem hiding this comment.
| project_owner = "MetOffice" | |
| PROJECT_OWNER = "MetOffice" |
gh_review_project/review_project.py
Outdated
| raise RuntimeError( | ||
| "Error fetching GitHub Project data: \n " + output.stderr.decode() | ||
| ) | ||
| command = f"gh project item-list {project_id} -L 500 --owner {project_owner} --format json" |
There was a problem hiding this comment.
It feels like project_id/owner should be inputs to this function, rather than hard-coded to the global variable.
Maybe def from_github(cls, project_id=PROJECT_ID, project_owner=PROJECT_OWNER...)?
There was a problem hiding this comment.
given there is much in this code that is specific to this project, I've left it just pulling directly from the globals for now since there are bigger changes needed to make this an option.
gh_review_project/review_project.py
Outdated
| """ | ||
|
|
||
| data = defaultdict(list) | ||
| data = [] |
There was a problem hiding this comment.
Would pull_requests or pull_requests_data be a clearer name?
There was a problem hiding this comment.
I guess same with self.data? If it's not just pr's then fair enough
gh_review_project/review_project.py
Outdated
| dry_run: If true, print the command used rather than archiving. | ||
| """ | ||
|
|
||
| command = f"gh project item-archive {project_id} --owner {project_owner} --id {self.id}" |
There was a problem hiding this comment.
Similar comments around project_owner/id as above
| pr.archive(dry_run) | ||
|
|
||
|
|
||
| class PullRequest: |
There was a problem hiding this comment.
Probably wants a docstring
There was a problem hiding this comment.
yep. Hopefully doc strings everywhere now?
gh_review_project/review_project.py
Outdated
| self.scitechReview = None | ||
| self.codeReview = None | ||
|
|
||
| def archive(self, dry_run: bool = False): |
There was a problem hiding this comment.
| def archive(self, dry_run: bool = False): | |
| def archive(self, dry_run: bool = False) -> None: |
| exceptions. | ||
| """ | ||
| total_open = still_open(open_prs[milestone], milestone) | ||
| total_other = closed_other(closed_prs, milestone) |
There was a problem hiding this comment.
Do we want to update the milestone on these PRs to match the closing milestone? I guess you'd logically do it after the check_ready step and before report?
There was a problem hiding this comment.
I didn't think I could, but worked it out now. Added to the closed_other function as no harm in doing this even if you're not ready to fully archive things.
This also had the side effect of reading things directly from the data everywhere as the modify_milestone function changes things so the copies I'd taken of open and closed PRs by milestone were then out of date.
| * Remaining open PRs and issues against this milestone | ||
| * Closed PRs against this milestone | ||
| """ | ||
| from collections import defaultdict |
There was a problem hiding this comment.
Not sure you're using this in this file?
james-bruten-mo
left a comment
james-bruten-mo
left a comment
There was a problem hiding this comment.
Thanks Jenny, changes look good
2 participants
PR Summary
Sci/Tech Reviewer:
Code Reviewer: @james-bruten-mo
New script for closing a milestone from the Review Tracker project during a release. It:
To do this it also improves the project data class, simplifying the data structure, including functions for handling milestones and storing the PR details in objects rather than a dictionary.
The user will need to authenticate themselves to modify the project, github prompts for this on first use.
Code Quality Checklist
Testing
Security Considerations
AI Assistance and Attribution
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review